-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make autoreleasepool take the pool as a parameter #103
base: master
Are you sure you want to change the base?
Conversation
The lifetime of the parameter is the lifetime of references in the pool. This allows us to bound lifetimes on autoreleased objects, thereby making them safe to return.
A nightly-only feature that adds the auto trait `AutoreleaseSafe` and uses it to prevent calling `autoreleasepool` with closures that capture an outer `AutoreleasePool`. This fixes on nightly the unsoundness hole preventing `autoreleasepool` from being used to safely construct references to autoreleased objects.
a1b5455
to
0d256c3
Compare
Prevent using the lifetime from an outer pool when an inner pool is active.
This is a lot of complexity for a nightly-only feature to allow safer autoreleases. I'm not sure why we need autoreleases to be safer. I'd say in rust people should only be using retain and release. Regardless, I'm not sure this would be worthwhile until the necessary features stabilize. |
To elaborate on this:
My understanding of autorelease is that it's predominantly used when returning an object so that it's the caller's responsibility to keep the object alive as long as it needs; you won't return them a +1 retain count object that they forget to release and now you've caused a memory leak in their code. I don't see how this problem applies to rust with its ownership system: I can return a smart pointer that will release the object when the caller is done with it and it drops. This seems like a strictly better and more idiomatic solution in rust, so I don't see why safe rust code should be using autorelease. |
The primary motivator for this is to make a safe function using The documentation on
What this means is that the lifetime of the returned pointer is either the same as the For reference, extern crate objc;
extern crate objc_foundation;
use objc::rc::autoreleasepool;
use objc_foundation::{NSString, INSObject, INSString};
fn main() {
let nsstring = NSString::from_str("String with non-ascii characters: 🔨");
let s = autoreleasepool(|| {
nsstring.as_str()
});
// `s` is invalid here
println!("{}", s);
} Running this with Guard Malloc (environment variable: Also, Secondary motivator is to be able to return autoreleased references for functions that inherently does that, like |
A simpler (but also more restricting) version than all the thread local stuff in debug mode could also just be to use some other unsafe auto trait that // Nightly w. feature
pub fn autoreleasepool<'p, T>(f: impl FnOnce(&'p AutoreleasePool) -> T + AutoreleaseSafe) { ... }
// Stable
pub fn autoreleasepool<'p, T>(f: impl FnOnce(&'p AutoreleasePool) -> T + Sync) { ... } // Or `Send` |
See the code comments and my explanation here: SSheldon/rust-objc#103 (comment) This also has the nice "side-effect" of fixing the memory leaks that as_str was otherwise exhibiting when using non-ascii strings, see SSheldon/rust-objc-foundation#15.
The pool is given as a reference, and the lifetime of the reference is the same as the lifetime of autoreleased objects in the pool. This allows us to bound the lifetimes of autoreleased objects, thereby making them safe to return.
Extracted from #98, see #95 for some more examples and the reasoning behind this.
UnsoundnessFixed.This is unsound until we can prevent people from doing something like:
I think this is possible to statically prevent using the nightlyDone!auto_traits
+negative_impls
features.It might also be possible to make a stable implementation forDone!debug_assertions
builds that panic if it happens.